Skip to content

Conversation

@vng
Copy link
Contributor

@vng vng commented Nov 30, 2025

Fixed bug with non-integer (or negative) lane tags.

Right now, osrm-extract crashes on Switzerland.

@TheMarex TheMarex requested a review from Copilot January 8, 2026 21:55
Copy link
Member

@TheMarex TheMarex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensible hardening. Looks good to me.

@TheMarex TheMarex merged commit 8dfc6bf into master Jan 8, 2026
23 checks passed
@TheMarex TheMarex deleted the vng-guidance branch January 8, 2026 21:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a crash in osrm-extract when processing OpenStreetMap data with invalid lane tags (non-integer or negative values), specifically addressing crashes when processing Switzerland data.

Key changes:

  • Added to_number_uint function to validate lane count values as positive integers
  • Replaced all tonumber calls for lane-related tag parsing with the new validation function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

'emergency_access'
}

local function to_number_uint(s)
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name to_number_uint suggests it returns an unsigned integer, but the implementation rejects 0 with the condition n > 0. In most programming contexts, unsigned integers include 0 (e.g., range is 0 to 2^32-1). While rejecting 0 is correct for lane counts (a road cannot have 0 lanes), consider naming it to_positive_int or to_number_positive_int for clarity.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants